[ci:fix] Improved changelog bot prompt (clarified commit message expectations)#657
Merged
nemesifier merged 7 commits intoopenwisp:masterfrom May 4, 2026
Merged
Conversation
…ectations, plain #issue references, and the proposed change log entry prefix
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 Walkthrough📝 Walkthrough🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
Files Reviewed (13 files)
Fix these issues in Kilo Cloud Reviewed by kimi-k2.5 · 682,408 tokens |
coderabbitai
Bot
added
github_actions
|
coverage: 97.348% (-0.2%) from 97.529% — pushpitkamboj:fix/changelog_system_prompt into openwisp:master |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/bot-changelog-generator/generate_changelog.py:
- Line 40: The constant COMMIT_SUBJECT_LIMIT is being reused for body wrapping
which is misleading; split it into two constants (COMMIT_SUBJECT_LIMIT and
COMMIT_BODY_WRAP_LIMIT) or rename to a neutral name (e.g., COMMIT_LINE_LIMIT)
and update all usages accordingly: keep COMMIT_SUBJECT_LIMIT for subject-length
checks, introduce COMMIT_BODY_WRAP_LIMIT for the prompt text that says "Wrap the
body around {COMMIT_SUBJECT_LIMIT} characters per line" (and any body-wrapping
logic), and change any places that validate or format the body to reference
COMMIT_BODY_WRAP_LIMIT so intent is unambiguous (search for COMMIT_SUBJECT_LIMIT
and the prompt string to locate affected code).
- Around line 393-412: The current loop treats any body line in
nonempty_body_lines that starts with a closing verb and contains "#" as a footer
and may reject the whole output; instead, detect the trailing footer block (the
last contiguous group of nonempty_body_lines at the end) and only validate lines
in that footer block against ISSUE_FOOTER_RE. Change the logic around
footer_lines/nonempty_body_lines so you scan nonempty_body_lines from the end to
find the start index of the footer block, collect only those lines into
footer_lines, and then run ISSUE_FOOTER_RE.fullmatch on each footer line
(returning False only if a footer-line fails validation); do not treat earlier
body lines as footers. Use the existing symbols footer_lines,
nonempty_body_lines and ISSUE_FOOTER_RE to locate and update the code paths.
In @.github/actions/bot-changelog-generator/test_generate_changelog.py:
- Around line 668-675: The test test_rejects_comment_intro_text is currently
failing for the wrong reason because the sample text fails the initial
tag/structure guards before the new CHANGELOG_COMMENT_INTRO check is exercised;
update the test to use a string that passes validate_changelog_output’s initial
checks (e.g., begins with a valid required tag and valid length/structure) but
still contains CHANGELOG_COMMENT_INTRO (case-insensitive) somewhere in the body
so that validate_changelog_output reaches and rejects based on the new
intro-check branch; refer to validate_changelog_output, required_tags and
CHANGELOG_COMMENT_INTRO to craft the input.
- Around line 626-651: The three tests (test_rejects_rst_issue_link_syntax,
test_rejects_markdown_issue_link_syntax,
test_rejects_parenthesized_pr_reference) currently fail earlier due to
title/footer mismatch; update each test's input passed to
validate_changelog_output to include a matching footer referencing the same
issue (e.g., a "Closes `#123`" or "Fixes `#123`" line in the body/footer) so the
only remaining validation that can reject is the disallowed_link_patterns check
in validate_changelog_output (and ensure you do not change the test names or the
expected assertFalse).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 47a5b631-6745-4ed4-a7cb-18f2137b9368
📒 Files selected for processing (2)
.github/actions/bot-changelog-generator/generate_changelog.py.github/actions/bot-changelog-generator/test_generate_changelog.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Kilo Code Review
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:10.320Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/bot-changelog-generator/generate_changelog.py:356-364
Timestamp: 2026-03-05T09:59:22.581Z
Learning: In `.github/actions/bot-changelog-generator/generate_changelog.py`, the `validate_changelog_output` function's purpose is to act as an output safety filter — ensuring no sensitive information or arbitrary LLM-generated text gets posted as a PR comment. It checks that the output starts with a valid tag ([feature]/[fix]/[change]) and contains a correctly structured PR reference pattern. It is NOT intended to strictly validate that the referenced PR number/URL matches the current PR.
📚 Learning: 2026-03-05T09:59:15.097Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/bot-changelog-generator/generate_changelog.py:356-364
Timestamp: 2026-03-05T09:59:15.097Z
Learning: In generate_changelog.py, reviews should confirm that validate_changelog_output acts as an output safety filter: it should require the produced changelog text to begin with a valid tag ([feature], [fix], or [change]) and to include a correctly formed PR reference pattern. It should NOT require that the referenced PR number or URL exactly match the current PR. For review, check: (1) the first non-space token matches one of the allowed tags, (2) the PR reference pattern is present and well-formed (e.g., contains a recognizable PR identifier or URL), and (3) there are no additional hard-coded cross-checks that would make it overly strict. This pattern should apply to this file specifically and guide future changes to similar output-filter functions without assuming cross-repo constraints.
Applied to files:
.github/actions/bot-changelog-generator/generate_changelog.py
📚 Learning: 2026-03-05T09:59:22.581Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/bot-changelog-generator/generate_changelog.py:356-364
Timestamp: 2026-03-05T09:59:22.581Z
Learning: In `.github/actions/bot-changelog-generator/generate_changelog.py`, the `validate_changelog_output` function's purpose is to act as an output safety filter — ensuring no sensitive information or arbitrary LLM-generated text gets posted as a PR comment. It checks that the output starts with a valid tag ([feature]/[fix]/[change]) and contains a correctly structured PR reference pattern. It is NOT intended to strictly validate that the referenced PR number/URL matches the current PR.
Applied to files:
.github/actions/bot-changelog-generator/test_generate_changelog.py
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Changes: Update tests to cover non-trivial changes and ensure proper validation of modified behavior
Applied to files:
.github/actions/bot-changelog-generator/test_generate_changelog.py
📚 Learning: 2026-02-04T07:19:40.541Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/changelog-generator/test_generate_changelog.py:4-22
Timestamp: 2026-02-04T07:19:40.541Z
Learning: In `.github/actions/changelog-generator/test_generate_changelog.py`, the sys.path manipulation before imports and use of absolute imports is intentional and preferred for readability, even though relative imports could work.
Applied to files:
.github/actions/bot-changelog-generator/test_generate_changelog.py
📚 Learning: 2026-03-05T09:38:10.320Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:10.320Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
Applied to files:
.github/actions/bot-changelog-generator/test_generate_changelog.py
🔇 Additional comments (3)
.github/actions/bot-changelog-generator/generate_changelog.py (2)
277-329: Prompt rewrite cleanly captures the PR objectives.The system instruction now enforces a plain-text git commit message with a tagged subject ≤
COMMIT_SUBJECT_LIMIT, blank-line-separated user-focused body, plain#123references with matchingCloses#123-style footers, and explicit prohibitions of RST/Markdown link syntax, GitHub URLs, code fences, and the `Proposed change log entry:` intro. This aligns with what `build_github_comment` and `validate_changelog_output` expect downstream, and the inline `example` is internally consistent with the rules (subject 54 chars, wrapped body, `Closes `#39footer matching the title's#39).
456-462: Centralizing comment construction inbuild_github_commentis a good refactor.The helper produces a deterministic comment body (marker + intro +
```textfence) which is also straightforward to assert against in tests. Wrapping the entry in atextfence preserves exact whitespace/line breaks so maintainers can copy it as a squash-merge commit message verbatim. Sincevalidate_changelog_outputalready rejects entries containing```, the inner content cannot break the outer fence..github/actions/bot-changelog-generator/test_generate_changelog.py (1)
406-414:TestBuildGithubCommentcovers the essentials.The assertions check the marker, the standardized intro, the
textcode-fence opener, and that the entry content is preserved — sufficient to catch regressions in the new helper.
|
|
||
| CHANGELOG_BOT_MARKER = "<!-- openwisp-changelog-bot -->" | ||
| CHANGELOG_COMMENT_INTRO = "Proposed change log entry:" | ||
| COMMIT_SUBJECT_LIMIT = 72 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
COMMIT_SUBJECT_LIMIT is reused for body wrapping — name is misleading.
The constant is named for the subject limit, but it is also threaded through the prompt to wrap body lines (line 309: Wrap the body around {COMMIT_SUBJECT_LIMIT} characters per line). Standard git commit convention separates these: ~50 chars for the subject, 72 for body wrap. If you intentionally use one limit for both, rename the constant to something neutral (e.g., COMMIT_LINE_LIMIT) or split into COMMIT_SUBJECT_LIMIT / COMMIT_BODY_WRAP_LIMIT so the intent is unambiguous to future readers.
♻️ Suggested split
-COMMIT_SUBJECT_LIMIT = 72
+COMMIT_SUBJECT_LIMIT = 72
+COMMIT_BODY_WRAP_LIMIT = 72And update the prompt body-wrap line to use COMMIT_BODY_WRAP_LIMIT.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/actions/bot-changelog-generator/generate_changelog.py at line 40,
The constant COMMIT_SUBJECT_LIMIT is being reused for body wrapping which is
misleading; split it into two constants (COMMIT_SUBJECT_LIMIT and
COMMIT_BODY_WRAP_LIMIT) or rename to a neutral name (e.g., COMMIT_LINE_LIMIT)
and update all usages accordingly: keep COMMIT_SUBJECT_LIMIT for subject-length
checks, introduce COMMIT_BODY_WRAP_LIMIT for the prompt text that says "Wrap the
body around {COMMIT_SUBJECT_LIMIT} characters per line" (and any body-wrapping
logic), and change any places that validate or format the body to reference
COMMIT_BODY_WRAP_LIMIT so intent is unambiguous (search for COMMIT_SUBJECT_LIMIT
and the prompt string to locate affected code).
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/bot-changelog-generator/generate_changelog.py:
- Around line 393-410: The code contains dead/redundant logic in the
footer-detection block: remove the unnecessary initialization footer_lines = []
and the second validation loop that iterates "for line in footer_lines: if not
ISSUE_FOOTER_RE.fullmatch(line): return False" because footer_lines is computed
from nonempty_body_lines[footer_start:] after a reverse scan that already
guarantees every footer_lines element matches ISSUE_FOOTER_RE; keep the reverse
scan that updates footer_start and the subsequent has_summary_line check, and
ensure any references to footer_lines are updated or removed accordingly in the
function handling nonempty_body_lines and ISSUE_FOOTER_RE.
In @.github/actions/bot-changelog-generator/test_generate_changelog.py:
- Around line 406-414: The test currently uses assertIn checks which allow
reordering or missing closing fence; update
TestBuildGithubComment.test_adds_intro_text_and_code_fence to verify the exact
structure produced by build_github_comment: construct the original entry string
(e.g., "[feature] Add feature\n\nBody text"), call build_github_comment(entry),
then assert the result equals the expected concatenation of
CHANGELOG_BOT_MARKER, CHANGELOG_COMMENT_INTRO, the opening "```text" fence, the
entry, and the closing "```" in the correct order; reference the
build_github_comment function and the CHANGELOG_BOT_MARKER and
CHANGELOG_COMMENT_INTRO constants when implementing the stricter assertion.
- Around line 599-633: The tests meant to exercise the suspicious_patterns
branch are passing for the wrong reason because validate_changelog_output
rejects these cases earlier due to a title↔footer issue mismatch; update each
failing test (test_rejects_prompt_injection_ignore_instructions,
test_rejects_prompt_injection_system, test_rejects_script_injection,
test_rejects_javascript_uri) so they isolate the suspicious_patterns check by
removing the unrelated footer "Closes `#123`" from the test body (or alternatively
add a matching "#123" reference in the title) so the validation reaches the
suspicious_patterns loop in generate_changelog.py (the code path that checks
ignore_[a-z_]*instructions, system\s*: , <script, javascript: patterns) and will
fail if those patterns are removed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ac5b0b5b-cf3d-42e0-a4d8-a532db0cbf9c
📒 Files selected for processing (2)
.github/actions/bot-changelog-generator/generate_changelog.py.github/actions/bot-changelog-generator/test_generate_changelog.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Kilo Code Review
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:10.320Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/bot-changelog-generator/generate_changelog.py:356-364
Timestamp: 2026-03-05T09:59:22.581Z
Learning: In `.github/actions/bot-changelog-generator/generate_changelog.py`, the `validate_changelog_output` function's purpose is to act as an output safety filter — ensuring no sensitive information or arbitrary LLM-generated text gets posted as a PR comment. It checks that the output starts with a valid tag ([feature]/[fix]/[change]) and contains a correctly structured PR reference pattern. It is NOT intended to strictly validate that the referenced PR number/URL matches the current PR.
📚 Learning: 2026-03-05T09:59:15.097Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/bot-changelog-generator/generate_changelog.py:356-364
Timestamp: 2026-03-05T09:59:15.097Z
Learning: In generate_changelog.py, reviews should confirm that validate_changelog_output acts as an output safety filter: it should require the produced changelog text to begin with a valid tag ([feature], [fix], or [change]) and to include a correctly formed PR reference pattern. It should NOT require that the referenced PR number or URL exactly match the current PR. For review, check: (1) the first non-space token matches one of the allowed tags, (2) the PR reference pattern is present and well-formed (e.g., contains a recognizable PR identifier or URL), and (3) there are no additional hard-coded cross-checks that would make it overly strict. This pattern should apply to this file specifically and guide future changes to similar output-filter functions without assuming cross-repo constraints.
Applied to files:
.github/actions/bot-changelog-generator/generate_changelog.py
📚 Learning: 2026-03-05T09:38:10.320Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:10.320Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
Applied to files:
.github/actions/bot-changelog-generator/generate_changelog.py.github/actions/bot-changelog-generator/test_generate_changelog.py
📚 Learning: 2026-02-10T20:38:27.593Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 593
File: openwisp_utils/releaser/commitizen.py:5-9
Timestamp: 2026-02-10T20:38:27.593Z
Learning: In openwisp-utils commitizen implementation, the `_TITLE_ISSUE_EXTRACT_RE` pattern intentionally matches any space-preceded `#<number>` in commit titles without anchoring to the end. This is acceptable because 99.9% of such patterns are actual issue references, and rare edge cases (like version numbers) will be handled manually if they occur.
Applied to files:
.github/actions/bot-changelog-generator/generate_changelog.py.github/actions/bot-changelog-generator/test_generate_changelog.py
📚 Learning: 2026-03-05T09:59:22.581Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/bot-changelog-generator/generate_changelog.py:356-364
Timestamp: 2026-03-05T09:59:22.581Z
Learning: In `.github/actions/bot-changelog-generator/generate_changelog.py`, the `validate_changelog_output` function's purpose is to act as an output safety filter — ensuring no sensitive information or arbitrary LLM-generated text gets posted as a PR comment. It checks that the output starts with a valid tag ([feature]/[fix]/[change]) and contains a correctly structured PR reference pattern. It is NOT intended to strictly validate that the referenced PR number/URL matches the current PR.
Applied to files:
.github/actions/bot-changelog-generator/test_generate_changelog.py
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Changes: Update tests to cover non-trivial changes and ensure proper validation of modified behavior
Applied to files:
.github/actions/bot-changelog-generator/test_generate_changelog.py
📚 Learning: 2026-02-04T07:19:40.541Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/changelog-generator/test_generate_changelog.py:4-22
Timestamp: 2026-02-04T07:19:40.541Z
Learning: In `.github/actions/changelog-generator/test_generate_changelog.py`, the sys.path manipulation before imports and use of absolute imports is intentional and preferred for readability, even though relative imports could work.
Applied to files:
.github/actions/bot-changelog-generator/test_generate_changelog.py
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Bug Fixes: Ensure the test is deterministic and not flaky - flag tests that depend on timing, sleeps, specific timezones, system time, randomness without fixed seed, race conditions, concurrency timing, network access, external services, filesystem state, environment-specific configuration, execution order, shared global state, hardcoded ports, or unawaited async operations
Applied to files:
.github/actions/bot-changelog-generator/test_generate_changelog.py
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Bug Fixes: Ensure the regression test fails if the patch is removed or commented out
Applied to files:
.github/actions/bot-changelog-generator/test_generate_changelog.py
🔇 Additional comments (3)
.github/actions/bot-changelog-generator/test_generate_changelog.py (1)
335-347: LGTM!The new assertions correctly validate the prompt's shift to a plain-text commit message format with a
COMMIT_SUBJECT_LIMIT-derived subject cap and an explicit prohibition on RST/Markdown link syntax. They match the strings emitted inbuild_prompt..github/actions/bot-changelog-generator/generate_changelog.py (2)
38-46: LGTM!Module constants and
ISSUE_FOOTER_REare well-scoped. The regex is intentionally strict (single space, no trailing punctuation, optional space-separated additional#Ns), which matches the system-prompt guidance shown to the model.
284-329: LGTM!The system instruction now clearly directs the model to emit a plain-text squash commit message with a tagged subject, blank-line body, plain
#Nreferences and matchingCloses/Fixes/...footers, and explicitly bans link/heading/code-fence syntax and the comment intro text. The wording is consistent with the newvalidate_changelog_outputrules.
Comment on lines
+406
to
+414
| class TestBuildGithubComment(unittest.TestCase): | ||
| """Tests for build_github_comment function.""" | ||
|
|
||
| def test_adds_intro_text_and_code_fence(self): | ||
| comment = build_github_comment("[feature] Add feature\n\nBody text") | ||
| self.assertIn(CHANGELOG_BOT_MARKER, comment) | ||
| self.assertIn(CHANGELOG_COMMENT_INTRO, comment) | ||
| self.assertIn("```text", comment) | ||
| self.assertIn("Body text", comment) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Tighten build_github_comment assertions to guard structure, not just substrings.
assertIn checks pass even if the marker, intro, and fenced block are reordered, the closing ``` is missing, or the entry isn't actually inside the fence. Consider asserting the full expected layout (or at least relative ordering and closing fence) so a regression in build_github_comment is caught.
♻️ Suggested stricter assertion
def test_adds_intro_text_and_code_fence(self):
- comment = build_github_comment("[feature] Add feature\n\nBody text")
- self.assertIn(CHANGELOG_BOT_MARKER, comment)
- self.assertIn(CHANGELOG_COMMENT_INTRO, comment)
- self.assertIn("```text", comment)
- self.assertIn("Body text", comment)
+ entry = "[feature] Add feature\n\nBody text"
+ comment = build_github_comment(entry)
+ expected = (
+ f"{CHANGELOG_BOT_MARKER}\n"
+ f"{CHANGELOG_COMMENT_INTRO}\n"
+ f"```text\n{entry}\n```"
+ )
+ self.assertEqual(comment, expected)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/actions/bot-changelog-generator/test_generate_changelog.py around
lines 406 - 414, The test currently uses assertIn checks which allow reordering
or missing closing fence; update
TestBuildGithubComment.test_adds_intro_text_and_code_fence to verify the exact
structure produced by build_github_comment: construct the original entry string
(e.g., "[feature] Add feature\n\nBody text"), call build_github_comment(entry),
then assert the result equals the expected concatenation of
CHANGELOG_BOT_MARKER, CHANGELOG_COMMENT_INTRO, the opening "```text" fence, the
entry, and the closing "```" in the correct order; reference the
build_github_comment function and the CHANGELOG_BOT_MARKER and
CHANGELOG_COMMENT_INTRO constants when implementing the stricter assertion.
nemesifier
reviewed
Apr 25, 2026
| " (concise for simple changes, more detailed if complex/relevant)\n" | ||
| "- On a new line, reference the PR number with a GitHub link\n\n" | ||
| "OUTPUT REQUIREMENTS:\n" | ||
| "- Start the first line with exactly one tag: [feature], [fix], or [change]\n" |
Member
There was a problem hiding this comment.
We support more than this. We should instruct the LLM to reuse the same prefix used in the PR title.
Contributor
Author
There was a problem hiding this comment.
as discussed in chat, we only support feature, fix, change
| f"- Wrap the body around {COMMIT_SUBJECT_LIMIT} characters per line\n" | ||
| "- If linked issues are present, use plain-text issue references such as " | ||
| "#123 in the title and matching footer lines such as Closes #123,\n" | ||
| " Fixes #123, Resolves #123, or Related to #123\n" |
Member
There was a problem hiding this comment.
Also here we should do the same: the PR description should already hint at what to use here, tell the LLM just reuse whathever is in the PR description or we'll get hallucinations.
Contributor
Author
There was a problem hiding this comment.
the issue number is itself comming from PR description, we are just telling how to show it. I can be more explicit to use PR description in prompt
| " the PR number as a substitute\n" | ||
| "- Do not use ReStructuredText/Markdown syntax to link issues\n" | ||
| "- Do not use GitHub URLs, PR links, code fences, or headings\n" | ||
| "- Do not add introductory text like 'Proposed change log entry:'\n\n" |
Member
There was a problem hiding this comment.
This is wrong:
Suggested change
| "- Do not add introductory text like 'Proposed change log entry:'\n\n" | |
| "- Add an introductory text like 'Proposed change log entry:'\n\n" |
I asked to add this preceding text because the bot comment looks totally out of the blue and without context, it's confusing for casual readers.
Contributor
Author
There was a problem hiding this comment.
yes, i have added it and instead of asking LLM to write, I wrote this line while building the final changelog, See line 453. Its better to have static behaviour for such things than asking LLM to write this line.
| "CHANGE TYPE TAGS (choose one):\n" | ||
| "- [feature] - New functionality\n" | ||
| "- [fix] - Bug fixes\n" | ||
| "- [change] - Non-breaking changes, refactors, updates\n" |
Member
There was a problem hiding this comment.
This part is wrong as explained earlier and also repeats something which is already explained above.
Contributor
Author
There was a problem hiding this comment.
as discussed in devs channel. We only support fix, feature, change
| "Length: Keep the subject short, but provide enough body detail to help " | ||
| "a maintainer reuse the output as a high-quality squash merge commit " | ||
| "message.\n" | ||
| "Output ONLY the commit message. No explanations, " |
Member
There was a problem hiding this comment.
This contradicts the preceding comment.
There was a problem hiding this comment.
-
I have updated it for now, but I think it may be better to guide the model more explicitly with concrete examples of the expected comment format, rather than relying mainly on prompt wording. Please see lines 277 and 333.
-
A few-shot prompting approach may be a more effective and reliable way to achieve the desired output format.
| CHANGELOG_COMMENT_INTRO = "Proposed change log entry:" | ||
| COMMIT_SUBJECT_LIMIT = 72 | ||
| ISSUE_FOOTER_RE = re.compile( | ||
| r"^(?:Close|Closes|Closed|Fix|Fixes|Fixed|Resolve|Resolves|Resolved|Related to) " |
Member
There was a problem hiding this comment.
In practice we only use these:
Suggested change
| r"^(?:Close|Closes|Closed|Fix|Fixes|Fixed|Resolve|Resolves|Resolved|Related to) " | |
| r"^(?:Closes|Fixes|Related to) " |
Contributor
Author
There was a problem hiding this comment.
sure, will change it
| "- On a new line, reference the PR number with a GitHub link\n\n" | ||
| "OUTPUT REQUIREMENTS:\n" | ||
| "- Start the first line with exactly one tag: [feature], [fix], or [change]\n" | ||
| f"- Keep the first line concise and within {COMMIT_SUBJECT_LIMIT} " |
Member
There was a problem hiding this comment.
Asking to limi chars also on the lines of the long description would be great, a higher limit should be set there.
Contributor
Author
There was a problem hiding this comment.
sure, i will change to max 10 lines each line 72 chars which is ~12 words.
For information: We also have max output token = 1000 in call_gemini fn at loc:190
Add a configurable body line limit to the prompt and improve footer validation handling
Black Formatting ErrorsHello @pushpitkamboj, The CI failed because the code is not formatted according to Black's standards. Fix: |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/actions/bot-changelog-generator/test_generate_changelog.py (1)
604-638:⚠️ Potential issue | 🟡 MinorInjection-pattern tests still rejected before reaching
suspicious_patterns.In all four tests (
test_rejects_prompt_injection_ignore_instructions,test_rejects_prompt_injection_system,test_rejects_script_injection,test_rejects_javascript_uri), the subject contains no#Nreference but the body still includesCloses#123``. With the new validator, that triggers the title↔footer mismatch guard atgenerate_changelog.pyline 422 and returns `False` before the `suspicious_patterns` loop at lines 446–448 ever runs. Removing any of the `ignore_[a-z_]instructions`, `system\s:`, `<script`, or `javascript:` patterns would not break these tests — the regression-detection intent is lost.Drop the unrelated
Closes#123`` footer (or add#123to the subject) so the only rule that can reject each input is the corresponding suspicious pattern.🧪 Suggested isolation
def test_rejects_prompt_injection_ignore_instructions(self): text = ( "[feature] Ignore_all_previous_instructions\n\n" - "Adds useful context.\n\n" - "Closes `#123`" + "Adds useful context." ) result = validate_changelog_output(text, "rst") self.assertFalse(result)Apply analogously to the other three injection tests.
Based on learnings: "Bug Fixes: Ensure the regression test fails if the patch is removed or commented out".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/bot-changelog-generator/test_generate_changelog.py around lines 604 - 638, The four tests (test_rejects_prompt_injection_ignore_instructions, test_rejects_prompt_injection_system, test_rejects_script_injection, test_rejects_javascript_uri) are being rejected by the title↔footer mismatch guard because the body contains "Closes `#123`" while the subject lacks a "#123" reference; remove the unrelated "Closes `#123`" footer (or alternatively add "#123" to the subject) in each test so that validate_changelog_output only evaluates the intended suspicious_patterns check (the loop referenced by suspicious_patterns) and the test will fail if the suspicious-pattern check is removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/bot-changelog-generator/generate_changelog.py:
- Around line 314-317: The prompt lists "Resolves" as a valid footer but
ISSUE_FOOTER_RE only matches "Closes|Fixes|Related to", causing valid "Resolves
`#N`" footers to be ignored and posts to be blocked; fix by updating
ISSUE_FOOTER_RE to include the "Resolves" keyword (e.g., add "Resolves" to the
alternation) so trailing-footer detection, footer_issues extraction, and the
title↔footer guard work correctly, or alternatively remove "Resolves" from the
user-facing prompt so it matches the current ISSUE_FOOTER_RE behavior (choose
one consistent approach across the prompt text and ISSUE_FOOTER_RE).
---
Duplicate comments:
In @.github/actions/bot-changelog-generator/test_generate_changelog.py:
- Around line 604-638: The four tests
(test_rejects_prompt_injection_ignore_instructions,
test_rejects_prompt_injection_system, test_rejects_script_injection,
test_rejects_javascript_uri) are being rejected by the title↔footer mismatch
guard because the body contains "Closes `#123`" while the subject lacks a "#123"
reference; remove the unrelated "Closes `#123`" footer (or alternatively add
"#123" to the subject) in each test so that validate_changelog_output only
evaluates the intended suspicious_patterns check (the loop referenced by
suspicious_patterns) and the test will fail if the suspicious-pattern check is
removed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 57ec9d0f-9e9d-4bde-ab82-1687e52e1f5c
📒 Files selected for processing (2)
.github/actions/bot-changelog-generator/generate_changelog.py.github/actions/bot-changelog-generator/test_generate_changelog.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Kilo Code Review
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:10.320Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/bot-changelog-generator/generate_changelog.py:356-364
Timestamp: 2026-03-05T09:59:22.581Z
Learning: In `.github/actions/bot-changelog-generator/generate_changelog.py`, the `validate_changelog_output` function's purpose is to act as an output safety filter — ensuring no sensitive information or arbitrary LLM-generated text gets posted as a PR comment. It checks that the output starts with a valid tag ([feature]/[fix]/[change]) and contains a correctly structured PR reference pattern. It is NOT intended to strictly validate that the referenced PR number/URL matches the current PR.
📚 Learning: 2026-03-05T09:59:15.097Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/bot-changelog-generator/generate_changelog.py:356-364
Timestamp: 2026-03-05T09:59:15.097Z
Learning: In generate_changelog.py, reviews should confirm that validate_changelog_output acts as an output safety filter: it should require the produced changelog text to begin with a valid tag ([feature], [fix], or [change]) and to include a correctly formed PR reference pattern. It should NOT require that the referenced PR number or URL exactly match the current PR. For review, check: (1) the first non-space token matches one of the allowed tags, (2) the PR reference pattern is present and well-formed (e.g., contains a recognizable PR identifier or URL), and (3) there are no additional hard-coded cross-checks that would make it overly strict. This pattern should apply to this file specifically and guide future changes to similar output-filter functions without assuming cross-repo constraints.
Applied to files:
.github/actions/bot-changelog-generator/generate_changelog.py
📚 Learning: 2026-03-05T09:38:10.320Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:10.320Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
Applied to files:
.github/actions/bot-changelog-generator/generate_changelog.py.github/actions/bot-changelog-generator/test_generate_changelog.py
📚 Learning: 2026-02-10T20:38:27.593Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 593
File: openwisp_utils/releaser/commitizen.py:5-9
Timestamp: 2026-02-10T20:38:27.593Z
Learning: In openwisp-utils commitizen implementation, the `_TITLE_ISSUE_EXTRACT_RE` pattern intentionally matches any space-preceded `#<number>` in commit titles without anchoring to the end. This is acceptable because 99.9% of such patterns are actual issue references, and rare edge cases (like version numbers) will be handled manually if they occur.
Applied to files:
.github/actions/bot-changelog-generator/generate_changelog.py.github/actions/bot-changelog-generator/test_generate_changelog.py
📚 Learning: 2026-03-05T09:59:22.581Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/bot-changelog-generator/generate_changelog.py:356-364
Timestamp: 2026-03-05T09:59:22.581Z
Learning: In `.github/actions/bot-changelog-generator/generate_changelog.py`, the `validate_changelog_output` function's purpose is to act as an output safety filter — ensuring no sensitive information or arbitrary LLM-generated text gets posted as a PR comment. It checks that the output starts with a valid tag ([feature]/[fix]/[change]) and contains a correctly structured PR reference pattern. It is NOT intended to strictly validate that the referenced PR number/URL matches the current PR.
Applied to files:
.github/actions/bot-changelog-generator/test_generate_changelog.py
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Changes: Update tests to cover non-trivial changes and ensure proper validation of modified behavior
Applied to files:
.github/actions/bot-changelog-generator/test_generate_changelog.py
📚 Learning: 2026-02-04T07:19:40.541Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/changelog-generator/test_generate_changelog.py:4-22
Timestamp: 2026-02-04T07:19:40.541Z
Learning: In `.github/actions/changelog-generator/test_generate_changelog.py`, the sys.path manipulation before imports and use of absolute imports is intentional and preferred for readability, even though relative imports could work.
Applied to files:
.github/actions/bot-changelog-generator/test_generate_changelog.py
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Bug Fixes: Ensure the regression test fails if the patch is removed or commented out
Applied to files:
.github/actions/bot-changelog-generator/test_generate_changelog.py
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/actions/bot-changelog-generator/generate_changelog.py (1)
313-316:⚠️ Potential issue | 🔴 CriticalPrompt still tells the LLM to emit
Resolves#N``, butISSUE_FOOTER_RErejects it — valid generations will be silently dropped.The system prompt on line 316 lists
Resolves#123as a valid footer keyword, but `ISSUE_FOOTER_RE` at lines 42–45 only accepts `Closes|Fixes|Related to`. When the model follows the prompt and emits a `Resolves `#Nfooter:
- The trailing-footer scan (lines 399–404) doesn't recognize it, so it's treated as a body summary line and not collected into
footer_lines.footer_issuesends up empty whiletitle_issuescontainsNfrom the subject, so the title↔footer guard at line 421–422 returnsFalse.main()logs"Possible prompt injection attempt detected. Skipping post."and the comment is never posted.Per the project convention discussed earlier (
Closes|Fixes|Related toonly), dropResolvesfrom the prompt to keep prompt and validator in sync.🛡️ Suggested prompt fix
- "- Use the linked issues we have from PR description provided. If linked issues \n" - "are present, use plain-text issue references such as \n" - "#123 in the title and matching footer lines such as Closes `#123`,\n" - " Fixes `#123`, Resolves `#123`, or Related to `#123`\n" + "- Use the linked issues from the PR description if present. When linked\n" + " issues are present, use plain-text issue references such as `#123` in\n" + " the title and matching footer lines such as Closes `#123`, Fixes `#123`,\n" + " or Related to `#123`\n"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/bot-changelog-generator/generate_changelog.py around lines 313 - 316, The system prompt lists "Resolves `#123`" as a valid footer but the validator regex ISSUE_FOOTER_RE (symbol) only accepts "Closes|Fixes|Related to", causing valid outputs to be rejected; update the prompt text in generate_changelog.py (the long prompt string that currently mentions "Resolves `#123`") to remove "Resolves" so the allowed footer examples match ISSUE_FOOTER_RE, ensuring title/footer detection and the title↔footer guard (used in main()) behave consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/actions/bot-changelog-generator/generate_changelog.py:
- Around line 313-316: The system prompt lists "Resolves `#123`" as a valid footer
but the validator regex ISSUE_FOOTER_RE (symbol) only accepts
"Closes|Fixes|Related to", causing valid outputs to be rejected; update the
prompt text in generate_changelog.py (the long prompt string that currently
mentions "Resolves `#123`") to remove "Resolves" so the allowed footer examples
match ISSUE_FOOTER_RE, ensuring title/footer detection and the title↔footer
guard (used in main()) behave consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e248a9a0-491e-4461-be67-bb9387a44346
📒 Files selected for processing (1)
.github/actions/bot-changelog-generator/generate_changelog.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Kilo Code Review
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:10.320Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/bot-changelog-generator/generate_changelog.py:356-364
Timestamp: 2026-03-05T09:59:22.581Z
Learning: In `.github/actions/bot-changelog-generator/generate_changelog.py`, the `validate_changelog_output` function's purpose is to act as an output safety filter — ensuring no sensitive information or arbitrary LLM-generated text gets posted as a PR comment. It checks that the output starts with a valid tag ([feature]/[fix]/[change]) and contains a correctly structured PR reference pattern. It is NOT intended to strictly validate that the referenced PR number/URL matches the current PR.
📚 Learning: 2026-03-05T09:59:15.097Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/bot-changelog-generator/generate_changelog.py:356-364
Timestamp: 2026-03-05T09:59:15.097Z
Learning: In generate_changelog.py, reviews should confirm that validate_changelog_output acts as an output safety filter: it should require the produced changelog text to begin with a valid tag ([feature], [fix], or [change]) and to include a correctly formed PR reference pattern. It should NOT require that the referenced PR number or URL exactly match the current PR. For review, check: (1) the first non-space token matches one of the allowed tags, (2) the PR reference pattern is present and well-formed (e.g., contains a recognizable PR identifier or URL), and (3) there are no additional hard-coded cross-checks that would make it overly strict. This pattern should apply to this file specifically and guide future changes to similar output-filter functions without assuming cross-repo constraints.
Applied to files:
.github/actions/bot-changelog-generator/generate_changelog.py
📚 Learning: 2026-03-05T09:38:10.320Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/workflows/reusable-bot-changelog.yml:49-49
Timestamp: 2026-03-05T09:38:10.320Z
Learning: In openwisp-utils, PR title prefixes are strictly limited to `[feature]`, `[fix]`, and `[change]` (exact bracketed tags, no scoping/sub-types). The regex `^\[(feature|fix|change)\]` in `.github/workflows/reusable-bot-changelog.yml` is intentional and correct — scoped variants like `[feature/bots]` are not valid and should not be matched.
Applied to files:
.github/actions/bot-changelog-generator/generate_changelog.py
📚 Learning: 2026-02-04T07:19:40.541Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/changelog-generator/test_generate_changelog.py:4-22
Timestamp: 2026-02-04T07:19:40.541Z
Learning: In `.github/actions/changelog-generator/test_generate_changelog.py`, the sys.path manipulation before imports and use of absolute imports is intentional and preferred for readability, even though relative imports could work.
Applied to files:
.github/actions/bot-changelog-generator/generate_changelog.py
📚 Learning: 2026-02-10T20:38:27.593Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 593
File: openwisp_utils/releaser/commitizen.py:5-9
Timestamp: 2026-02-10T20:38:27.593Z
Learning: In openwisp-utils commitizen implementation, the `_TITLE_ISSUE_EXTRACT_RE` pattern intentionally matches any space-preceded `#<number>` in commit titles without anchoring to the end. This is acceptable because 99.9% of such patterns are actual issue references, and rare edge cases (like version numbers) will be handled manually if they occur.
Applied to files:
.github/actions/bot-changelog-generator/generate_changelog.py
nemesifier
requested changes
Apr 27, 2026
|
|
||
| def test_invalid_no_tag(self): | ||
| text = "Added new functionality\n\nAdds useful context.\n\nCloses #123" | ||
| result = validate_changelog_output(text, "rst") |
Member
There was a problem hiding this comment.
These tests are not needed if the reuse the work done for commitizen, this is mostly bloat.
| r"\[#\d+\]\(https?://[^\)]+\)", | ||
| r"\(#\d+\)", | ||
| r"https?://github\.com/[^)\s>]+/(?:pull|issues)/\d+", | ||
| ] |
Member
There was a problem hiding this comment.
We have logic in this repo to validate that the commit message is good:
- https://github.com/openwisp/openwisp-utils/blob/master/openwisp_utils/releaser/commitizen.py
- https://github.com/openwisp/openwisp-utils/blob/master/docs/developer/qa-checks.rst#commit-message-checks
There's no need to reintroduce duplicated logic here, can you call the commitizen plugin instead?
To the LLM prompt I think we should give it these files as contex, saying the LLM must follow these rules for validating the commit message produced:
- https://github.com/openwisp/openwisp-utils/blob/master/openwisp_utils/releaser/commitizen.py
- https://github.com/openwisp/openwisp-utils/blob/master/openwisp_utils/releaser/tests/test_commitizen_rules.py
What happens if validation fails?
Contributor
Author
|
@CodeRabbit-ai review it |
|
Tip For best results, initiate chat on the files or code changes.
[review] |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/actions/bot-changelog-generator/test_generate_changelog.py (2)
421-426: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winStrengthen wrapper-structure assertion to exact match.
Current
assertInchecks won’t catch reordering or a missing closing code fence. Assert the full expected comment string instead.♻️ Proposed test tightening
def test_adds_intro_text_and_code_fence(self): - comment = build_github_comment("[feature] Add feature\n\nBody text") - self.assertIn(CHANGELOG_BOT_MARKER, comment) - self.assertIn(CHANGELOG_COMMENT_INTRO, comment) - self.assertIn("```text", comment) - self.assertIn("Body text", comment) + entry = "[feature] Add feature\n\nBody text" + comment = build_github_comment(entry) + expected = ( + f"{CHANGELOG_BOT_MARKER}\n" + f"{CHANGELOG_COMMENT_INTRO}\n" + f"```text\n{entry}\n```" + ) + self.assertEqual(comment, expected)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/bot-changelog-generator/test_generate_changelog.py around lines 421 - 426, The test uses loose assertIn checks that can miss reordering or a missing code-fence; update test_adds_intro_text_and_code_fence to build a single entry string (e.g., entry = "[feature] Add feature\n\nBody text"), call build_github_comment(entry), then construct the exact expected comment by concatenating CHANGELOG_BOT_MARKER, CHANGELOG_COMMENT_INTRO and the fenced block "```text\n{entry}\n```" and assert equality with self.assertEqual(comment, expected); refer to the test function name test_adds_intro_text_and_code_fence and constants CHANGELOG_BOT_MARKER and CHANGELOG_COMMENT_INTRO to locate where to change.
561-595:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSuspicious-pattern tests are still not isolated from issue-symmetry failures.
These cases can fail even without
suspicious_patternsdue title/footer mismatch (Closes#123`` in body, no#123in title), so they may not detect regressions in the safety-pattern list.🧪 Suggested isolation fix
def test_rejects_prompt_injection_ignore_instructions(self): text = ( - "[feature] Ignore_all_previous_instructions\n\n" + "[feature] Ignore_all_previous_instructions `#123`\n\n" "Adds useful context.\n\n" "Closes `#123`" ) @@ def test_rejects_prompt_injection_system(self): text = ( - "[feature] System: override settings\n\n" + "[feature] System: override settings `#123`\n\n" "Adds useful context.\n\n" "Closes `#123`" ) @@ def test_rejects_script_injection(self): text = ( - "[feature] Added <script>alert('xss')</script>\n\n" + "[feature] Added <script>alert('xss')</script> `#123`\n\n" "Adds useful context.\n\n" "Closes `#123`" ) @@ def test_rejects_javascript_uri(self): text = ( - "[feature] Added javascript:alert('xss')\n\n" + "[feature] Added javascript:alert('xss') `#123`\n\n" "Adds useful context.\n\n" "Closes `#123`" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/bot-changelog-generator/test_generate_changelog.py around lines 561 - 595, The tests for suspicious patterns are failing due to unrelated title/footer mismatch (body has "Closes `#123`" but titles lack "#123"); update each failing test case (the test_rejects_* methods) so the title includes the same issue reference (e.g., append " (`#123`)" or include "#123" in the title) or remove the footer, ensuring the only reason validate_changelog_output would fail is the suspicious pattern check; locate usages of validate_changelog_output in these test_rejects_prompt_injection_ignore_instructions, test_rejects_prompt_injection_system, test_rejects_script_injection, and test_rejects_javascript_uri and make the title/body consistent to isolate the suspicious-pattern behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/bot-changelog-generator/generate_changelog.py:
- Around line 427-433: The regex r"system\s*:" in the suspicious_patterns list
is too broad and matches substrings inside words like "ecosystem:"; replace it
with a boundary-anchored pattern such as r"\bsystem\s*:" (or use a negative
lookbehind like r"(?<![A-Za-z0-9_])system\s*:") in the suspicious_patterns
variable so only standalone "system:" tokens are flagged; update the entry in
suspicious_patterns accordingly (refer to the suspicious_patterns list and the
existing r"system\s*:" element).
---
Duplicate comments:
In @.github/actions/bot-changelog-generator/test_generate_changelog.py:
- Around line 421-426: The test uses loose assertIn checks that can miss
reordering or a missing code-fence; update test_adds_intro_text_and_code_fence
to build a single entry string (e.g., entry = "[feature] Add feature\n\nBody
text"), call build_github_comment(entry), then construct the exact expected
comment by concatenating CHANGELOG_BOT_MARKER, CHANGELOG_COMMENT_INTRO and the
fenced block "```text\n{entry}\n```" and assert equality with
self.assertEqual(comment, expected); refer to the test function name
test_adds_intro_text_and_code_fence and constants CHANGELOG_BOT_MARKER and
CHANGELOG_COMMENT_INTRO to locate where to change.
- Around line 561-595: The tests for suspicious patterns are failing due to
unrelated title/footer mismatch (body has "Closes `#123`" but titles lack "#123");
update each failing test case (the test_rejects_* methods) so the title includes
the same issue reference (e.g., append " (`#123`)" or include "#123" in the title)
or remove the footer, ensuring the only reason validate_changelog_output would
fail is the suspicious pattern check; locate usages of validate_changelog_output
in these test_rejects_prompt_injection_ignore_instructions,
test_rejects_prompt_injection_system, test_rejects_script_injection, and
test_rejects_javascript_uri and make the title/body consistent to isolate the
suspicious-pattern behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e5577baf-ba59-4dfd-8528-d5b11dc3f3ba
📒 Files selected for processing (3)
.github/actions/bot-changelog-generator/action.yml.github/actions/bot-changelog-generator/generate_changelog.py.github/actions/bot-changelog-generator/test_generate_changelog.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Kilo Code Review
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-03-05T09:59:15.097Z
Learnt from: pushpitkamboj
Repo: openwisp/openwisp-utils PR: 584
File: .github/actions/bot-changelog-generator/generate_changelog.py:356-364
Timestamp: 2026-03-05T09:59:15.097Z
Learning: In generate_changelog.py, reviews should confirm that validate_changelog_output acts as an output safety filter: it should require the produced changelog text to begin with a valid tag ([feature], [fix], or [change]) and to include a correctly formed PR reference pattern. It should NOT require that the referenced PR number or URL exactly match the current PR. For review, check: (1) the first non-space token matches one of the allowed tags, (2) the PR reference pattern is present and well-formed (e.g., contains a recognizable PR identifier or URL), and (3) there are no additional hard-coded cross-checks that would make it overly strict. This pattern should apply to this file specifically and guide future changes to similar output-filter functions without assuming cross-repo constraints.
Applied to files:
.github/actions/bot-changelog-generator/generate_changelog.py
🔇 Additional comments (1)
.github/actions/bot-changelog-generator/action.yml (1)
29-32: Good dependency alignment for Commitizen validation.Explicitly pinning
commitizenkeeps runtime validation behavior predictable for this action.
Contributor
Author
|
@CodeRabbit-ai, please review again. if everything is good, approve it |
|
Tip For best results, initiate chat on the files or code changes.
[full_review] |
nemesifier
requested changes
May 4, 2026
| run: pip install "openwisp-utils[github_actions] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/master.tar.gz" | ||
| run: > | ||
| pip install | ||
| "openwisp-utils[github_actions] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/master.tar.gz" |
Member
There was a problem hiding this comment.
Suggested change
| "openwisp-utils[github_actions] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/master.tar.gz" | |
| "openwisp-utils[github_actions,qa] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/master.tar.gz" |
| run: > | ||
| pip install | ||
| "openwisp-utils[github_actions] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/master.tar.gz" | ||
| "commitizen>=4.13.0,<5.0.0" |
Member
There was a problem hiding this comment.
Suggested change
| "commitizen>=4.13.0,<5.0.0" |
|
|
||
| if CHANGELOG_COMMENT_INTRO.lower() in text.lower(): | ||
| errors.append("Commit message must not include the GitHub comment intro text.") | ||
|
|
|
|
||
| if "```" in text: | ||
| errors.append("Commit message must not contain fenced code blocks.") | ||
|
|
|
|
||
| if not any(text.startswith(tag) for tag in required_tags): | ||
| errors.append("Commit message must start with [feature], [fix], or [change].") | ||
|
|
nemesifier
approved these changes
May 4, 2026
[change] Improved the changelog bot system prompt to make the expected output more explicit, guiding it to generate a commit-message-style changelog suggestion with a concise subject and a clearer user-focused body. The bot will now use plain `#issue` references instead of Markdown or ReStructuredText issue links, and the PR comment format will prepend "Proposed change log entry:" before the generated suggestion. Validation and tests have been adjusted to match the new expected output format.
`#657 <https://github.com/openwisp/openwisp-utils/pull/657>`_ |
nemesifier
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
Reference to Existing Issue
Closes #656
Guides the bot to generate a commit-message-style changelog suggestion with a concise subject and a clearer user-focused body.